-
Notifications
You must be signed in to change notification settings - Fork 8.2k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[App Search] Convert Search UI view to new page template + minor UI polish #102813
Conversation
+ update tests TODO
- On a totally new engine, all pages except this one* had an empty state, so per Davey's recommendations I whipped up a new empty state for this page * Overview has a custom 'empty' state, analytics does not have an empty state
<p> | ||
{i18n.translate('xpack.enterpriseSearch.appSearch.engine.searchUI.empty.description', { | ||
defaultMessage: | ||
'A schema will be automatically created for you after you index some documents.', |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This copy is the exact same as the empty states for relevance tuning & result settings, since all of these pages depend on/use engine schema
<AppSearchPageTemplate | ||
pageChrome={getEngineBreadcrumbs([SEARCH_UI_TITLE])} | ||
pageHeader={{ pageTitle: SEARCH_UI_TITLE }} | ||
isEmptyState={Object.keys(engine.schema || {}).length === 0} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Heads up, I fully intend on refactoring this Object.keys
check to an EngineLogic selector - but that's going to come in a follow-up polish PR once all the views land. Essentially I want to tackle the concept of empty engine polling more holistically and at the engine logic level, and ensure all pages update dynamically when an engine is empty and receives new documents/schema.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Also, I did check w/ Davey on this and he confirmed that this view should check for empty schema and not just for empty documents. So someone could index a document and create a schema but then delete the document, and this would no longer show the empty state (which matches how Relevance Tuning & Result Settings behave)
...plugins/enterprise_search/public/applications/app_search/components/engine/engine_router.tsx
Outdated
Show resolved
Hide resolved
💚 Build SucceededMetrics [docs]Module Count
Async chunks
History
To update your PR or re-run it, just comment with: |
💚 Backport successful
This backport PR will be merged automatically after passing CI. |
…olish (elastic#102813) * Convert Search UI view to use new page template + update tests TODO * [UX polish] Add empty state to Search UI view - On a totally new engine, all pages except this one* had an empty state, so per Davey's recommendations I whipped up a new empty state for this page * Overview has a custom 'empty' state, analytics does not have an empty state * Update router * Fix bad merge conflict resolution * [Polish] Copy feedback proposed by Davey - see elastic@cbc3706
…olish (#102813) (#102952) * Convert Search UI view to use new page template + update tests TODO * [UX polish] Add empty state to Search UI view - On a totally new engine, all pages except this one* had an empty state, so per Davey's recommendations I whipped up a new empty state for this page * Overview has a custom 'empty' state, analytics does not have an empty state * Update router * Fix bad merge conflict resolution * [Polish] Copy feedback proposed by Davey - see cbc3706 Co-authored-by: Constance <constancecchen@users.noreply.github.com>
Summary
Follow up to #102170 - converts more App Search pages to the new KibanaPageTemplate. I'm attempting to break up the AS layout conversion into smaller, easier to review chunks.
This PR handles the Search UI view, and additionally adds a new empty state for the page (discussed and approved by Davey last week). As always, follow along by commit (and turn off whitespace diffs)
Screencaps
New empty state:
Checklist